-
Notifications
You must be signed in to change notification settings - Fork 340
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Admin panel allow all emails option #41
Conversation
d0eff80
to
f360600
Compare
Add allow/dissallow all emails button in the admin settings panel.
The canRegister function now checks the allowAllEmails flag in the settings model when a user registers. A false allowAllEmails flag continues to check the whitelisted emails while a true allowAllEmails flag will allow the user to register with any valid email. Moved the email validator before the settings check.
f360600
to
8cd2d36
Compare
Awesome! Can we start a place to document all the new settings in the admin panel (either in the |
Huh - I just realized that we had a change that we made for blueprint to allow for all emails that didn't get propagated up to the main repo. There, we added a special case for the '*' selector in whitelisted emails, and we can accomplish all of this behavior server-side. UX-wise, I don't like the idea of having both the toggle and whitelisted emails available as an option, since it can become unclear as to which option has precedence. |
@ehzhang If would like to keep the 'allow all emails' toggle, I can hide/disable the whitelist section once it's toggled, to indicate that they are mutually exclusive. Let me know! @jlin816 |
Hey Jeremy -- sorry for the delay! Let me know whether this is an intuitive UI! |
Sounds great! I think that does make it more clear. I will update the documentation to reflect this new setting as well. |
@Jeremy091 Bumping this! Are you still planning to finish up this PR? |
@Jeremy091 any updates with this PR? |
Revisiting old PRs I had open. Closing since it is very stale. |
Issue #15
Closes #15
This PR is dependent on #40, since it builds on code in that PR.
Making another contribution for Hacktoberfest, which completes #15.
I added a new checkbox called 'Allow all emails' to the 'Additional Options' section on the admin settings page.
Toggling this will update a new boolean property called
allowAllEmails
in the server's Settings schema,and update the settings client side. A 'put' endpoint was added to the server api.js to allow changes to the
allowAllEmails
setting.Upon new user registration, the UserController validation that occurs server side now checks the settings for
allowAllEmails
flag after validating that the email is a valid email type. IfallowAllEmails
is set totrue
, then the whitelist will not be checked for valid educational emails.If the
allowAllEmails
flag is set tofalse
, then the whitelist validation proceeds as normal.I did my best to remain consistent with the codebase, and added as little as possible to get this working. Please don't hesitate to let me know if you'd like this done differently. Thanks!
Example Screenshots:
Additional options section.
Allow all emails set to disallowed.
This is what it looks like when toggled to 'allowed'.
This is the confirmation that pops up after you toggle the checkbox.